From 3dc4be051c0a9d05d795030873ebdb0537a78bc1 Mon Sep 17 00:00:00 2001 From: tsteven4 <13596209+tsteven4@users.noreply.github.com> Date: Mon, 20 Jan 2020 10:09:52 -0700 Subject: [PATCH] Datetime optimization (#439) * Optimize performance of DateTime. Prefer Qt::UTC TimeSpec for storing creation_time. This avoids very significant performance hits from converting to/from Qt::LocalTime, which involve DST calculations as well as the usual offsets. Test suite with this commit runs at 63% of wall-clock time of the baseline (time ./testo)! * remove obsolete fromTime_t and use Qt::UTC. * Delete obsolete operators and methods of DateTime. * document motivation for Qt::UTC as default timespec. Modify Waypoint::SetCreationTime so it can be used with seconds, milliseconds, or both operands. This method is implemented without resetting the Qt::TimeSpec. It will be efficient if the existing TimeSpec is Qt::UTC as set up by gpsbabel::DateTime default constructor. --- defs.h | 3 +-- garmin_fit.cc | 2 +- gopal.cc | 11 ++++------- gtm.cc | 4 ++-- lowranceusr.cc | 2 +- random.cc | 4 ++-- src/core/datetime.h | 37 ++++++++++--------------------------- unicsv.cc | 8 ++++---- util.cc | 4 ++-- waypt.cc | 11 ++--------- xcsv.cc | 8 ++++---- 11 files changed, 33 insertions(+), 61 deletions(-) diff --git a/defs.h b/defs.h index c906bc458..8c4055347 100644 --- a/defs.h +++ b/defs.h @@ -552,8 +552,7 @@ public: QString CreationTimeXML() const; gpsbabel::DateTime GetCreationTime() const; void SetCreationTime(const gpsbabel::DateTime& t); - void SetCreationTime(time_t t); - void SetCreationTime(time_t t, int ms); + void SetCreationTime(qint64 t, qint64 ms = 0); geocache_data* AllocGCData(); int EmptyGCData() const; }; diff --git a/garmin_fit.cc b/garmin_fit.cc index 357484b76..ca55c6ffb 100644 --- a/garmin_fit.cc +++ b/garmin_fit.cc @@ -779,7 +779,7 @@ fit_parse_data(fit_message_def* def, int time_offset) if (alt != 0xffff) { waypt->altitude = (alt / 5.0) - 500; } - waypt->SetCreationTime(QDateTime::fromTime_t(GPS_Math_Gtime_To_Utime(timestamp))); + waypt->SetCreationTime(GPS_Math_Gtime_To_Utime(timestamp)); if (speed != 0xffff) { WAYPT_SET(waypt, speed, speed / 1000.0f); } diff --git a/gopal.cc b/gopal.cc index 31f52b872..c3c74ba9e 100644 --- a/gopal.cc +++ b/gopal.cc @@ -177,9 +177,7 @@ gopal_read() route_head* route = route_head_alloc(); - QDateTime qtx; - qtx.setTimeSpec(Qt::UTC); - qtx.setTime_t(tx); + QDateTime qtx = QDateTime::fromSecsSinceEpoch(tx, Qt::UTC); route->rte_name = "Tracklog "; route->rte_name += qtx.toString(Qt::ISODate); route_add_head(route); @@ -290,7 +288,7 @@ gopal_read() if (!strptime(c, "%Y%m%d", &tm2)) { fatal("Bad date '%s'.\n", c); } - wpt->creation_time += mkgmtime(&tm2); + wpt->creation_time = wpt->creation_time.addSecs(mkgmtime(&tm2)); break; case 10: // Unknown. Ignored. case 11: // Bearing. Ignored. @@ -335,11 +333,10 @@ gopal_read() static void gopal_write_waypt(const Waypoint* wpt) { - char tbuffer[64]; int fix=fix_unknown; //TICK; TIME; LONG; LAT; HEIGHT; SPEED; UN; HDOP; SAT //3801444, 080558, 2.944362, 43.262117, 295.28, 0.12964, 2, 2.900000, 3 - snprintf(tbuffer, sizeof(tbuffer), "%06d", wpt->creation_time.hms()); + QString tbuffer = wpt->creation_time.toString("HHmmss"); if (wpt->fix!=fix_unknown) { switch (wpt->fix) { case fix_none: @@ -355,7 +352,7 @@ gopal_write_waypt(const Waypoint* wpt) } //MSVC handles time_t as int64, gcc and mac only int32, so convert it: unsigned long timestamp = (unsigned long)wpt->GetCreationTime().toTime_t(); - gbfprintf(fout, "%lu, %s, %lf, %lf, %5.1lf, %8.5lf, %d, %lf, %d\n",timestamp,tbuffer, wpt->longitude, wpt->latitude,wpt->altitude, + gbfprintf(fout, "%lu, %s, %lf, %lf, %5.1lf, %8.5lf, %d, %lf, %d\n",timestamp, CSTR(tbuffer), wpt->longitude, wpt->latitude,wpt->altitude, wpt->speed,fix,wpt->hdop,wpt->sat); } diff --git a/gtm.cc b/gtm.cc index f417fd630..1f0175145 100644 --- a/gtm.cc +++ b/gtm.cc @@ -539,7 +539,7 @@ gtm_read() fread_discard(file_in, 1); wpt->SetCreationTime(fread_long(file_in)); if (wpt->creation_time.isValid()) { - wpt->creation_time += EPOCH89DIFF; + wpt->creation_time = wpt->creation_time.addSecs(EPOCH89DIFF); } fread_discard(file_in, 2); wpt->altitude = fread_single(file_in); @@ -567,7 +567,7 @@ gtm_read() convert_datum(&wpt->latitude, &wpt->longitude); wpt->SetCreationTime(fread_long(file_in)); if (wpt->creation_time.isValid()) { - wpt->creation_time += EPOCH89DIFF; + wpt->creation_time = wpt->creation_time.addSecs(EPOCH89DIFF); } start_new = fread_byte(file_in); wpt->altitude = fread_single(file_in); diff --git a/lowranceusr.cc b/lowranceusr.cc index 6c656854d..6435b5864 100644 --- a/lowranceusr.cc +++ b/lowranceusr.cc @@ -1537,7 +1537,7 @@ lowranceusr4_parse_trail(int* trail_num) gbfgetc(file_in); /* POSIX timestamp (a.k.a. UNIX Epoch) - seconds since Jan 1, 1970 */ - wpt_tmp->SetCreationTime(QDateTime::fromTime_t(gbfgetint32(file_in))); + wpt_tmp->SetCreationTime(gbfgetint32(file_in)); /* Long/Lat */ wpt_tmp->longitude = gbfgetdbl(file_in) / DEGREESTORADIANS; /* rad to deg */ diff --git a/random.cc b/random.cc index fd14816d3..c94b05375 100644 --- a/random.cc +++ b/random.cc @@ -235,7 +235,7 @@ random_read() route_head* head; Waypoint* prev = nullptr; - QDateTime time = current_time(); + QDateTime time = current_time().toUTC(); int points = (opt_points) ? atoi(opt_points) : rand_int(128) + 1; if (doing_trks || doing_rtes) { @@ -286,7 +286,7 @@ random_rd_posn_init(const QString&) if (opt_points) { realtime->points = atoi(opt_points); } - realtime->time = current_time(); + realtime->time = current_time().toUTC(); } void diff --git a/src/core/datetime.h b/src/core/datetime.h index 7c7eeff78..0c7581b3a 100644 --- a/src/core/datetime.h +++ b/src/core/datetime.h @@ -42,38 +42,21 @@ class DateTime : public QDateTime { public: // As a crutch, mimic the old behaviour of an uninitialized creation time // being 1/1/1970. - DateTime() { - setTime_t(0); + // The choice of Qt::TimeSpec here can be critical to performance. + // With a Qt::LocalTime conversions between Qt::UTC and Qt::LocalTime + // can be required when using a method such as *Epoch, add*. + // Using Qt::Utc avoids these conversions. + // The lowranceusr regression test was measured taking 2.7x longer, + // and the entire regression suite took 1.7x longer, with + // Qt::LocalTime compared to Qt::UTC on ubuntu bionic. + // Note that these conversions can be required if the Qt::TimeSpec is + // set to Qt:LocalTime after construction. + DateTime() : QDateTime(QDateTime::fromMSecsSinceEpoch(0, Qt::UTC)) { } DateTime(const QDate& date, const QTime& time) : QDateTime(date, time) {} DateTime(const QDateTime& dt) : QDateTime(dt) {} - // TODO: this should go away in favor of .addSecs(). - // add time_t without losing any existing milliseconds. - DateTime& operator+=(const time_t& t) { - QDateTime dt = addSecs(t); - setDate(dt.date()); - setTime(dt.time()); - return *this; - } - - // Integer form: YYMMDD - int ymd() const { - QDate d(date()); - return d.year() * 10000 + d.month() * 100 + d.day(); - } - - int ddmmyy() const { - QDate d(date()); - return d.day() * 10000 + d.month() * 100 + d.year(); - } - - int hms() const { - QTime t(time()); - return t.hour() * 10000 + t.minute() * 100 + t.second(); - } - // Temporary: Override the standard, also handle time_t 0 as invalid. bool isValid() const { return date().isValid() && time().isValid() && toTime_t() > 0; diff --git a/unicsv.cc b/unicsv.cc index 2c75836e4..b9c2f2654 100644 --- a/unicsv.cc +++ b/unicsv.cc @@ -468,7 +468,7 @@ unicsv_adjust_time(const time_t time, const time_t* date) struct tm tm = *gmtime(&res); res = mklocaltime(&tm); } - return QDateTime::fromTime_t(res); + return QDateTime::fromSecsSinceEpoch(res, Qt::UTC); } static bool @@ -1131,7 +1131,7 @@ unicsv_parse_one_line(const QString& ibuf) } if (opt_utc) { - wpt->creation_time += atoi(opt_utc) * SECONDS_PER_HOUR; + wpt->creation_time = wpt->creation_time.addSecs(atoi(opt_utc) * SECONDS_PER_HOUR); } } @@ -1658,7 +1658,7 @@ unicsv_waypt_disp_cb(const Waypoint* wpt) // We might wrap to a different day by overriding the TZ offset. dt = dt.addSecs(atoi(opt_utc) * SECONDS_PER_HOUR); } else { - dt = wpt->GetCreationTime(); + dt = wpt->GetCreationTime().toLocalTime(); } QString date = dt.toString("yyyy/MM/dd"); *fout << unicsv_fieldsep << date; @@ -1673,7 +1673,7 @@ unicsv_waypt_disp_cb(const Waypoint* wpt) t = wpt->GetCreationTime().toUTC().time(); t = t.addSecs(atoi(opt_utc) * SECONDS_PER_HOUR); } else { - t = wpt->GetCreationTime().time(); + t = wpt->GetCreationTime().toLocalTime().time(); } QString out; if (t.msec() > 0) { diff --git a/util.cc b/util.cc index eae14c614..ded89eefa 100644 --- a/util.cc +++ b/util.cc @@ -735,10 +735,10 @@ gpsbabel::DateTime current_time() { if (gpsbabel_testmode()) { - return QDateTime::fromTime_t(0); + return QDateTime::fromMSecsSinceEpoch(0, Qt::UTC); } - return QDateTime::currentDateTime(); + return QDateTime::currentDateTimeUtc(); } /* diff --git a/waypt.cc b/waypt.cc index 2f8bfb5f2..67921e8ac 100644 --- a/waypt.cc +++ b/waypt.cc @@ -595,16 +595,9 @@ Waypoint::SetCreationTime(const gpsbabel::DateTime& t) } void -Waypoint::SetCreationTime(time_t t) +Waypoint::SetCreationTime(qint64 t, qint64 ms) { - creation_time = QDateTime::fromTime_t(t); -} - -void -Waypoint::SetCreationTime(time_t t, int ms) -{ - creation_time.setTime_t(t); - creation_time = creation_time.addMSecs(ms); + creation_time.setMSecsSinceEpoch((t * 1000) + ms); } geocache_data* diff --git a/xcsv.cc b/xcsv.cc index 035771358..11d9c1ca5 100644 --- a/xcsv.cc +++ b/xcsv.cc @@ -806,7 +806,7 @@ xcsv_parse_val(const QString& value, Waypoint* wpt, const field_map& fmp, case XT_TIMET_TIME_MS: { /* Time as time_t in milliseconds */ bool ok; - wpt->SetCreationTime(QDateTime::fromMSecsSinceEpoch(value.toLongLong(&ok))); + wpt->SetCreationTime(0, value.toLongLong(&ok)); if (!ok) { warning("parse of string '%s' on line number %d as TIMET_TIME_MS failed.\n", s, line_no); } @@ -820,17 +820,17 @@ xcsv_parse_val(const QString& value, Waypoint* wpt, const field_map& fmp, break; case XT_LOCAL_TIME: if (!gpsbabel_testmode()) { - wpt->creation_time += sscanftime(s, fmp.printfc.constData(), 0); + wpt->creation_time = wpt->creation_time.addSecs(sscanftime(s, fmp.printfc.constData(), 0)); } else { /* Force constant time zone for test */ - wpt->creation_time += sscanftime(s, fmp.printfc.constData(), 1); + wpt->creation_time = wpt->creation_time.addSecs(sscanftime(s, fmp.printfc.constData(), 1)); } break; /* Useful when time and date are in separate fields GMT / Local offset is handled by the two cases above */ case XT_HMSG_TIME: case XT_HMSL_TIME: - wpt->creation_time += addhms(s, fmp.printfc.constData()); + wpt->creation_time = wpt->creation_time.addSecs(addhms(s, fmp.printfc.constData())); break; case XT_ISO_TIME: case XT_ISO_TIME_MS: -- 2.30.2